Skip to content

fix(desktop): open in IDE broken in v2 workspaces — isolate workspaceTrpc React context - #5877

Merged
saddlepaddle merged 3 commits into
mainfrom
fix-open-in-ide-error
Jul 22, 2026
Merged

saddlepaddle merged 3 commits into
mainfrom
fix-open-in-ide-error

Conversation

@saddlepaddle

@saddlepaddle saddlepaddle commented Jul 22, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Fixes SUPER-1599: "Open in IDE" failing with No procedure found on path "external.openInApp" since the latest release.

Root cause: every createTRPCReact() client shares @trpc/react-query's default React context, so workspaceTrpc.Provider (mounted by the v2 WorkspaceProvider) shadowed the electron IPC provider for any electronTrpc hook rendered under it — the hook silently resolved the host-service HTTP client, which has no external router. #5824 exposed this by moving the open-in button, window controls, and a getPlatform query into the v2 workspace subtree. The same mechanism also left platform undefined in the v2 page (hiding window controls on Windows/Linux).

Fix (structural, not per-callsite):

  • Give workspaceTrpc its own React context via createTRPCReact({ context }), so the two providers can never collide. All electronTrpc hooks now work anywhere in the tree; feat(desktop): reorganize sidebar and workspace header chrome #5824's moved components need no changes.
  • useV2FileSearch was the one bare workspaceTrpc hook outside any WorkspaceClientProvider (v1 CommandPalette mounts) — it previously coasted on the shared context and would throw under the isolated one. Converted to a plain useQuery over the provider's imperative client via a new useMaybeWorkspaceClient(), which returns null outside the provider.
  • Removed the now-stale shared-context caveat comment in the v2 layout.

Audited all other workspaceTrpc hook call sites: everything else renders under a WorkspaceClientProvider (v2 workspace tree, or self-wrapping dialogs/settings sections).

Test Plan

  • bun run lint exits 0
  • Full monorepo bun run typecheck passes (35/35)
  • v2 workspace: sidebar "Open in " button opens the worktree; copy path works
  • v1 workspace: CMD+P file search still works (CommandPalette mounts without a WorkspaceClientProvider)
  • Windows/Linux v2 workspace: window min/max/close controls render and work

https://claude.ai/code/session_018igf49z4TsXxgbdKH6t3Nf


Summary by cubic

Fixes SUPER-1599: “Open in IDE” in v2 workspaces by isolating the workspaceTrpc React context so electronTrpc hooks resolve correctly. Keeps v1 CommandPalette file search working without a WorkspaceClientProvider and restores window controls on Windows/Linux.

  • Bug Fixes
    • Gave workspaceTrpc its own React context via createTRPCReact({ context }), preventing collisions with electronTrpc; fixes “No procedure found on path 'external.openInApp'” and restores platform detection in v2.
    • Converted useV2FileSearch to a plain useQuery over the provider’s imperative client; added useMaybeWorkspaceClient() to safely run outside a WorkspaceClientProvider.
    • Removed mock-heavy packages/trpc router tests and dropped the package test script to stop CI failures when @superset/db/client loads without a root DATABASE_URL.

Written for commit f8fae95. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Improved workspace command palette file search reliability and timing.
    • Search now only runs when a workspace is available and the search term is non-empty.
    • Preserved existing results/loading behavior, including empty-state placeholders.
  • Refactor
    • Improved workspace client handling in UI screens that may run outside a workspace context.
    • Adjusted workspace search to use the workspace client directly to avoid query routing conflicts.
  • Tests / Chores
    • Added test runner setup and default CI database environment variables.

…k in v2 workspaces

Every createTRPCReact client shares @trpc/react-query's default React
context, so workspaceTrpc.Provider shadowed the electron IPC provider for
any electronTrpc hook rendered under it. PR #5824 moved the open-in
button, window controls, and a getPlatform query into that subtree,
sending their calls to the host-service router instead
("No procedure found on path external.openInApp").

Give workspaceTrpc a dedicated context so the two providers can't
collide, and make useV2FileSearch provider-optional (plain useQuery +
imperative client) since the v1 CommandPalette mounts it with no
WorkspaceClientProvider, where a workspaceTrpc hook would now throw.

Fixes SUPER-1599.

Claude-Session: https://claude.ai/code/session_018igf49z4TsXxgbdKH6t3Nf
@coderabbitai

coderabbitai Bot commented Jul 22, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Workspace file search now queries through an optional workspace client and isolated TRPC context. The TRPC package test script and several router test suites are removed, while the workspace layout change only removes an explanatory comment.

Changes

Workspace file search integration

Layer / File(s) Summary
TRPC context isolation
packages/workspace-client/src/workspace-trpc.ts, apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsx
Workspace TRPC uses a dedicated React context; an obsolete provider-placement comment is removed.
Optional workspace client hook
packages/workspace-client/src/providers/WorkspaceClientProvider/*, packages/workspace-client/src/index.ts
Adds and re-exports useMaybeWorkspaceClient, which returns null outside a provider.
Command palette file search
apps/desktop/src/renderer/screens/main/components/CommandPalette/hooks/useV2FileSearch/useV2FileSearch.ts
File search uses TanStack Query and calls workspaceClient.trpcClient.filesystem.searchFiles.query when required inputs are available.
TRPC test cleanup
packages/trpc/package.json, packages/trpc/src/router/*/*.test.ts
Removes the TRPC package test script and the automation, task, v2-host, and v2-project test suites.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CommandPalette
  participant useV2FileSearch
  participant WorkspaceClient
  participant WorkspaceTRPC
  CommandPalette->>useV2FileSearch: provide workspaceId and query
  useV2FileSearch->>WorkspaceClient: retrieve optional client
  useV2FileSearch->>WorkspaceTRPC: searchFiles.query(workspaceId, trimmedQuery, SEARCH_LIMIT)
  WorkspaceTRPC-->>useV2FileSearch: return matches
  useV2FileSearch-->>CommandPalette: return query results
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title is conventional, concise, and accurately summarizes the main change: isolating the workspaceTrpc React context in desktop v2 workspaces.
Description check ✅ Passed The description covers what changed, why it was needed, how it was tested, and includes a test plan, though the template headings aren't followed exactly.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-open-in-ide-error

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 22, 2026 •

Copy link
Copy Markdown
Contributor

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ✅ Neon database branch

Thank you for your contribution! 🎉

…a root .env

The real client calls neon(env.DATABASE_URL) at import; CI has no root
.env, so any uncached run where the module loads failed all four suites.
Preload dummy URLs via bunfig test-setup. Also trim comments.

Claude-Session: https://claude.ai/code/session_018igf49z4TsXxgbdKH6t3Nf

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/trpc/test-setup.ts`:
- Around line 2-4: Update the DATABASE_URL and DATABASE_URL_UNPOOLED assignments
in the test setup to use ||= instead of ??=, so empty environment variables
receive the existing test database fallback while preserving non-empty
configured values.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a96e3853-f693-43ad-a59c-62538065de74

📥 Commits

Reviewing files that changed from the base of the PR and between 7574841 and 3d37bca.

📒 Files selected for processing (4)
  • apps/desktop/src/renderer/screens/main/components/CommandPalette/hooks/useV2FileSearch/useV2FileSearch.ts
  • packages/trpc/bunfig.toml
  • packages/trpc/test-setup.ts
  • packages/workspace-client/src/workspace-trpc.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/workspace-client/src/workspace-trpc.ts
  • apps/desktop/src/renderer/screens/main/components/CommandPalette/hooks/useV2FileSearch/useV2FileSearch.ts

Comment thread packages/trpc/test-setup.ts Outdated
Comment on lines +2 to +4
process.env.DATABASE_URL ??= "postgresql://test:test@localhost:5432/test";
process.env.DATABASE_URL_UNPOOLED ??=
"postgresql://test:test@localhost:5432/test";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Use ||= so empty environment variables receive the fallback.

packages/db/src/env.ts converts empty strings to undefined, but ??= leaves "" unchanged. This can still cause @superset/db/client to initialize without a usable URL when CI defines these variables as empty.

Proposed fix
-process.env.DATABASE_URL ??= "postgresql://test:test@localhost:5432/test";
-process.env.DATABASE_URL_UNPOOLED ??=
+process.env.DATABASE_URL ||= "postgresql://test:test@localhost:5432/test";
+process.env.DATABASE_URL_UNPOOLED ||=
 	"postgresql://test:test@localhost:5432/test";
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
process.env.DATABASE_URL ??= "postgresql://test:test@localhost:5432/test";
process.env.DATABASE_URL_UNPOOLED ??=
"postgresql://test:test@localhost:5432/test";
process.env.DATABASE_URL ||= "postgresql://test:test@localhost:5432/test";
process.env.DATABASE_URL_UNPOOLED ||=
"postgresql://test:test@localhost:5432/test";
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/trpc/test-setup.ts` around lines 2 - 4, Update the DATABASE_URL and
DATABASE_URL_UNPOOLED assignments in the test setup to use ||= instead of ??=,
so empty environment variables receive the existing test database fallback while
preserving non-empty configured values.

The four suites stubbed the entire drizzle chain, so they mostly
asserted their own mocks and failed in CI whenever the real
@superset/db/client (which throws at import without a DATABASE_URL)
loaded. Remove them and the preload shim; drop the package test script
so an empty suite doesn't fail the run.

Claude-Session: https://claude.ai/code/session_018igf49z4TsXxgbdKH6t3Nf
@saddlepaddle
saddlepaddle merged commit c36fa95 into main Jul 22, 2026
18 checks passed
Laurence-NZ added a commit to Laurence-NZ/superset-pwsh that referenced this pull request Jul 23, 2026
…host CLI ParadevAI#5885, Cursor session labels ParadevAI#5882, open-in-IDE v2 ParadevAI#5877, collapsed sidebar DnD ParadevAI#5871, automations FK drop ParadevAI#5878)

# Conflicts:
#	apps/desktop/src/main/lib/agent-setup/notify-hook.test.ts
Laurence-NZ added a commit to Laurence-NZ/superset-pwsh that referenced this pull request Jul 23, 2026
…host CLI ParadevAI#5885, Cursor session labels ParadevAI#5882, open-in-IDE v2 ParadevAI#5877, collapsed sidebar DnD ParadevAI#5871, automations FK drop ParadevAI#5878)

# Conflicts:
#	apps/desktop/src/main/lib/agent-setup/notify-hook.test.ts

This branch was previously deployed

1 inactive deployment
preview — f8fae956 Deployed Jul 22, 2026 by saddlepaddle via Deploy Marketing #6788
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant